-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Canvas] Fix bug when trying to move elements #199211
[Canvas] Fix bug when trying to move elements #199211
Conversation
💚 Build Succeeded
Metrics [docs]Async chunks
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
code review only
Starting backport for target branches: 8.16, 8.18, 8.x |
Closes elastic#199110 ## Summary In elastic#194634, we switched to hard-coded strings for the reducers to prevent an import circular dependency - see https://github.com/elastic/kibana/pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120 for the relevant change. What we didn't realize at the time, however, is that there was actually a typo in the original creation of the set position action - the `actionType` was set to the singular `setMultiplePosition` rather than `setMultiplePositions`, so there was actually **no** reducer tied to the `setMultiplePositions` action type after our change - i.e. the reducer map was expecting `setMultiplePosition` and did nothing for `setMultiplePositions`. By changing the `actionType` to the proper plural `setMultiplePositions`, the reducer map now has a match, so the reducer gets called as expected. | Before | After | |--------|--------| | ![Nov-06-2024 14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3) | ![Nov-06-2024 14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08) | I looked through the other changes we made to reducer map keys, and every `actionType` defined via `createAction` seems to have a matching reducer map key - so this appears to be a one-off 🙈 ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 1fa3089)
Closes elastic#199110 ## Summary In elastic#194634, we switched to hard-coded strings for the reducers to prevent an import circular dependency - see https://github.com/elastic/kibana/pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120 for the relevant change. What we didn't realize at the time, however, is that there was actually a typo in the original creation of the set position action - the `actionType` was set to the singular `setMultiplePosition` rather than `setMultiplePositions`, so there was actually **no** reducer tied to the `setMultiplePositions` action type after our change - i.e. the reducer map was expecting `setMultiplePosition` and did nothing for `setMultiplePositions`. By changing the `actionType` to the proper plural `setMultiplePositions`, the reducer map now has a match, so the reducer gets called as expected. | Before | After | |--------|--------| | ![Nov-06-2024 14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3) | ![Nov-06-2024 14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08) | I looked through the other changes we made to reducer map keys, and every `actionType` defined via `createAction` seems to have a matching reducer map key - so this appears to be a one-off 🙈 ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) (cherry picked from commit 1fa3089)
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
# Backport This will backport the following commits from `main` to `8.x`: - [[Canvas] Fix bug when trying to move elements (#199211)](#199211) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-06T23:54:44Z","message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:small","impact:critical","v9.0.0","Feature:Canvas","backport:version","v8.17.0","v8.16.1","v8.18.0"],"title":"[Canvas] Fix bug when trying to move elements","number":199211,"url":"https://github.com/elastic/kibana/pull/199211","mergeCommit":{"message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.16","8.18"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199211","number":199211,"mergeCommit":{"message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Hannah Mudge <[email protected]>
# Backport This will backport the following commits from `main` to `8.16`: - [[Canvas] Fix bug when trying to move elements (#199211)](#199211) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Hannah Mudge","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-11-06T23:54:44Z","message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","Team:Presentation","loe:small","impact:critical","v9.0.0","Feature:Canvas","backport:version","v8.17.0","v8.16.1","v8.18.0"],"title":"[Canvas] Fix bug when trying to move elements","number":199211,"url":"https://github.com/elastic/kibana/pull/199211","mergeCommit":{"message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.16","8.18"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/199211","number":199211,"mergeCommit":{"message":"[Canvas] Fix bug when trying to move elements (#199211)\n\nCloses https://github.com/elastic/kibana/issues/199110\r\n\r\n## Summary\r\n\r\nIn #194634, we switched to\r\nhard-coded strings for the reducers to prevent an import circular\r\ndependency - see\r\nhttps://github.com//pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120\r\nfor the relevant change.\r\n\r\nWhat we didn't realize at the time, however, is that there was actually\r\na typo in the original creation of the set position action - the\r\n`actionType` was set to the singular `setMultiplePosition` rather than\r\n`setMultiplePositions`, so there was actually **no** reducer tied to the\r\n`setMultiplePositions` action type after our change - i.e. the reducer\r\nmap was expecting `setMultiplePosition` and did nothing for\r\n`setMultiplePositions`. By changing the `actionType` to the proper\r\nplural `setMultiplePositions`, the reducer map now has a match, so the\r\nreducer gets called as expected.\r\n\r\n| Before | After |\r\n|--------|--------|\r\n| ![Nov-06-2024\r\n14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3)\r\n| ![Nov-06-2024\r\n14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08)\r\n|\r\n\r\nI looked through the other changes we made to reducer map keys, and\r\nevery `actionType` defined via `createAction` seems to have a matching\r\nreducer map key - so this appears to be a one-off 🙈\r\n\r\n### Checklist\r\n\r\n- [x] This was checked for [cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n### For maintainers\r\n\r\n- [ ] This was checked for breaking API changes and was [labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels)\r\n- [ ] This will appear in the **Release Notes** and follow the\r\n[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)","sha":"1fa30899ab2ba3f534ebb6620a34513a1431e0ed"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.16","label":"v8.16.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.18","label":"v8.18.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Hannah Mudge <[email protected]>
Closes elastic#199110 ## Summary In elastic#194634, we switched to hard-coded strings for the reducers to prevent an import circular dependency - see https://github.com/elastic/kibana/pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120 for the relevant change. What we didn't realize at the time, however, is that there was actually a typo in the original creation of the set position action - the `actionType` was set to the singular `setMultiplePosition` rather than `setMultiplePositions`, so there was actually **no** reducer tied to the `setMultiplePositions` action type after our change - i.e. the reducer map was expecting `setMultiplePosition` and did nothing for `setMultiplePositions`. By changing the `actionType` to the proper plural `setMultiplePositions`, the reducer map now has a match, so the reducer gets called as expected. | Before | After | |--------|--------| | ![Nov-06-2024 14-04-37](https://github.com/user-attachments/assets/627a3fee-2835-446a-b949-f44632d797d3) | ![Nov-06-2024 14-05-19](https://github.com/user-attachments/assets/f0baed94-3858-47b7-b979-7f27deb50b08) | I looked through the other changes we made to reducer map keys, and every `actionType` defined via `createAction` seems to have a matching reducer map key - so this appears to be a one-off 🙈 ### Checklist - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#_add_your_labels) - [ ] This will appear in the **Release Notes** and follow the [guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
Closes #199110
Summary
In #194634, we switched to hard-coded strings for the reducers to prevent an import circular dependency - see https://github.com/elastic/kibana/pull/194634/files#diff-12e4182415d9eb779aadf492d2b777393e2794c7ac2eb39c48310ab6493ab233L115-R120 for the relevant change.
What we didn't realize at the time, however, is that there was actually a typo in the original creation of the set position action - the
actionType
was set to the singularsetMultiplePosition
rather thansetMultiplePositions
, so there was actually no reducer tied to thesetMultiplePositions
action type after our change - i.e. the reducer map was expectingsetMultiplePosition
and did nothing forsetMultiplePositions
. By changing theactionType
to the proper pluralsetMultiplePositions
, the reducer map now has a match, so the reducer gets called as expected.I looked through the other changes we made to reducer map keys, and every
actionType
defined viacreateAction
seems to have a matching reducer map key - so this appears to be a one-off 🙈Checklist
For maintainers